Skip to content

Conversation

@louis-jan
Copy link
Contributor

@louis-jan louis-jan commented Jul 1, 2025

Describe Your Changes

This PR aims to ensure that all model settings are sent to the llama.cpp server. In this patch, we'll apply a very minimal validation to avoid potential issues. Any enhancements should be submitted to the llama.cpp extension Epic.

With some of parameters disabled (leave empty)
CleanShot 2025-07-01 at 13 21 32@2x

With value inputed
CleanShot 2025-07-01 at 13 21 25@2x

Fixes Issues

  • Closes #
  • Closes #

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Important

Fixes issue where some model settings were not applied by converting specific parameters to float in normalizeValue() in utils.ts.

  • Behavior:
    • Ensures all model settings are sent to the llama.cpp server by updating normalizeValue() in utils.ts to convert specific parameters to float.
    • Applies minimal validation to prevent potential issues.
  • Functions:
    • Updates normalizeValue() to handle temperature, top_k, top_p, min_p, repeat_penalty, frequency_penalty, presence_penalty, repeat_last_n as floats.
    • Uses validationRules in extractInferenceParams() and extractModelLoadParams() to validate and normalize parameters.

This description was created by Ellipsis for db74f2c. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed everything up to 7b22ba8 in 1 minute and 24 seconds. Click for details.
  • Reviewed 27 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_khCKgKtAPCFqo3xP

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@LazyYuuki LazyYuuki added this to the v0.6.4 milestone Jul 1, 2025
@LazyYuuki LazyYuuki added this to Jan Jul 1, 2025
@LazyYuuki LazyYuuki moved this to In Progress in Jan Jul 1, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed db74f2c in 1 minute and 20 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_CQZmB3XYJLzXT37Y

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@louis-jan louis-jan merged commit 9e9bc49 into release/v0.6.4 Jul 1, 2025
13 checks passed
@louis-jan louis-jan deleted the fix/some-of-model-settings-are-not-applied branch July 1, 2025 06:43
@github-project-automation github-project-automation bot moved this from In Progress to QA in Jan Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants